Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ProtocolStart and GracefulClose messages #3839

Merged
merged 14 commits into from
Sep 22, 2023

Conversation

gregtatcam
Copy link
Collaborator

@gregtatcam gregtatcam commented May 6, 2021

High Level Overview of Change

Add InboundHandoff class to send HTTP response
to the outbound peer and instantiate inbound PeerImp
when the response is sent. The peer then waits for
mtSTART_PROTOCOL message from the connected
outbound peer. When the message is received, the peer starts
sending the protocol messages. This update ensures that
the inbound peer is created once the overlay is established,
eliminating the need for doAccept(). In addition it synchronizes
send/receive protocol messages between the peers on start,
eliminating the need for copying the read buffer into the outbound
peer constructor.

Add mtGRACEFUL_CLOSE message to differentiate application
and link layer failures.

Type of Change

  • Refactor (non-breaking change that only restructures code)
  • New Feature

Copy link
Contributor

@HowardHinnant HowardHinnant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ripple/overlay/impl/OverlayImpl.cpp needs an additional include to compile in a non-unity build:

#include <ripple/overlay/impl/InboundHandoff.h>

src/ripple/overlay/impl/InboundHandoff.cpp Show resolved Hide resolved
Copy link

@SirJosh1987 SirJosh1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help me I only have a phone

…on start.

Add InboundHandoff class to complete HTTP handshake and instantiate inbound PeerImp.
Add GracefulClose message to close a peer connection on an application level error.
the last message is graceful close or if
aborted and close flag is set.
Don't close right after sending graceful close.
[FOLD] Add required include for non-unity build.
Make InboundHandoff non-copiable.
@intelliot
Copy link
Collaborator

(notes: changes how servers communicate with each other; Nik will review later)

@intelliot intelliot added Request for Comments Feature Request Used to indicate requests to add new features labels Dec 23, 2022
@intelliot
Copy link
Collaborator

@JoelKatz FYI - since this is an area of the code that hasn't been touched in many years, let me know if there is anything that needs to be further tested.

@sophiax851 it would be useful to confirm that none of these changes have adverse performance impact. Thank you!

@sophiax851
Copy link
Collaborator

sophiax851 commented Sep 22, 2023

I'll add an item in our queue to run a comparison test with the 1.12 baseline. @intelliot

@intelliot
Copy link
Collaborator

Thank you! This will be included in the next beta. If we find any issues with it, we can revert it as appropriate.

@intelliot intelliot merged commit 8f89694 into XRPLF:develop Sep 22, 2023
15 checks passed
@intelliot
Copy link
Collaborator

note: we should check with @nbougalis on whether this should be kept in

Bronek added a commit to Bronek/rippled that referenced this pull request Dec 13, 2023
ximinez added a commit to ximinez/rippled that referenced this pull request Dec 14, 2023
ximinez added a commit to ximinez/rippled that referenced this pull request Dec 14, 2023
intelliot pushed a commit that referenced this pull request Dec 19, 2023
@intelliot intelliot mentioned this pull request Dec 20, 2023
2 tasks
@intelliot intelliot added the Reverted Changes which should still be considered for re-merging. See "Closed" PRs with this label label Jan 10, 2024
@intelliot intelliot modified the milestones: 2.0.0, refactors Jan 10, 2024
sophiax851 pushed a commit to sophiax851/rippled that referenced this pull request Jun 12, 2024
Clean up the peer-to-peer protocol start/close sequences by introducing
START_PROTOCOL and GRACEFUL_CLOSE messages, which sync inbound/outbound
peer send/receive. The GRACEFUL_CLOSE message differentiates application
and link layer failures.

* Introduce the `InboundHandoff` class to manage inbound peer
  instantiation and synchronize the send/receive protocol messages
  between peers.
* Update `OverlayImpl` to utilize the `InboundHandoff` class to manage
  inbound handshakes.
* Update `PeerImp` for improved handling of protocol messages.
* Modify the `Message` class for better maintainability.
* Introduce P2P protocol version `2.3`.
sophiax851 pushed a commit to sophiax851/rippled that referenced this pull request Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion Reverted Changes which should still be considered for re-merging. See "Closed" PRs with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants